fix(npm_and_yarn): split compound engine constraints before Requirement parsing#15115
fix(npm_and_yarn): split compound engine constraints before Requirement parsing#15115thavaahariharangit wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Improves npm_and_yarn engine constraint handling so valid npm-style Node engine ranges (including compound ranges produced by caret/tilde expansion and alternative ranges) don’t crash requirement parsing.
Changes:
- Normalize extracted engine constraints by splitting compound comparator strings into individual comparator tokens before building a
Dependabot::NpmAndYarn::Requirement. - Update logging to reflect the normalized constraint list passed to
Requirement.new. - Add a regression spec for a Node engines expression like
^22.0.0 || >=24that previously triggered an “Illformed requirement” error.
Show a summary per file
| File | Description |
|---|---|
| npm_and_yarn/lib/dependabot/npm_and_yarn/package_manager.rb | Splits compound engine constraint strings into parser-safe tokens before constructing Requirement. |
| npm_and_yarn/spec/dependabot/npm_and_yarn/package_manager_helper_spec.rb | Adds regression coverage for compound + alternative Node engine constraints. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 1
|
@thavaahariharangit is |
This is the package.json Ref: https://github.com/thavaahariharangit/dependabot-caret-engines-repro-FR-13667/blob/main/package.json
But dependabot is saying |
@thavaahariharangit I can see this fixes the issue, but I'm not sure what the side effects of this PR are. I'm guessing we're using these versions for corepack, so what version are we actually pulling in? |
|
covered here: #15144 |
What are you trying to accomplish?
This change makes Dependabot more robust when interpreting Node engine constraints so updates do not fail on valid npm-style range expressions.
Why:
Some projects specify Node support with compound and alternative ranges (for example, a bounded major range plus a newer-major fallback). Those constraints are valid in the ecosystem, but the updater could pass a combined range fragment into requirement parsing in a shape that triggered an Illformed requirement error. That caused update jobs to fail even though the manifest constraints were legitimate.
What changed at a high level:
The updater now normalizes parsed engine constraints into parser-safe comparator tokens before building the requirement object. In other words, compound ranges are preserved semantically, but represented in the granular form expected by the requirement parser.
Approach:
The fix is intentionally narrow and low-risk:
Result:
Valid Node engine expressions that include compound ranges and alternatives are now handled correctly, eliminating this failure mode and improving updater reliability for real-world package manifests.
Anything you want to highlight for special attention from reviewers?
Error reproduced in this workflow: https://github.com/thavaahariharangit/dependabot-caret-engines-repro-FR-13667/actions/runs/26219192273/job/77149420577
How will you know you've accomplished your goal?
Before this update:
After this update:
Checklist